Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[rlp-v2] WasmPlugin reconciliation #204

Merged
merged 15 commits into from
Jul 20, 2023
Merged

Conversation

guicassolato
Copy link
Contributor

@guicassolato guicassolato commented Jun 22, 2023

Reconciliation of the Istio WasmPlugin CR with wasm rules generated for each limit definition from the affecting RLPs – triggers based on routeSelectors and when conditions, and RL data (descriptors) to activate the limits in Limitador and additional counter qualifiers.

Implements Direct Policy Attachment of RLPs targeting a HTTPRoute or a Gateway (WasmPlugin part).

Highlights

  • Reconciliation of HTTPRoute RLPs → Wasm rule conditions generated out of the limit's when conditions and HTTPRouteRules selected by the routeSelectors, and data that activates the limit in Limitador plus additional counter qualifiers
  • Fix pattern expression for hostnames with the right operator (endswith)
  • Compute the domain intersection between the hostnames specified in the HTTPRoute and the hostnames specified in the Gateway, so it doesn’t generate wasm rules for the wrong gateway/hostname
  • Stop repeating RLPs for each hostname within the wasm config (see WASM Configuration v2 wasm-shim#35 (comment))
  • Reconciliation of Gateway RLPs → Fetch all HTTPRoutes accepted by the Gateway that do not have a RLP of its own, and then fake a HTTPRoute with all rules from those HTTPRoutes, to to generate sets of wasm rules (conditions and data) for each limit of the gateway RLP
  • Prevent use of routeSelectors in RLPs that target a Gateway
  • Stop merging gateway RLP rules into the route RLP configs – otherwise we would be implementing (partially) Inherited Policy Attachment instead.
  • Do not generate conditions for gateway policy wasm rules from HTTPRoutes that have a RLP of its own
  • Avoid building a wasm plugin config if there are no rules to apply

Verification steps

Topology:

          *                       *.internal
          │                           │
          ▼                           ▼
┌──────────────────────┐    ┌───────────────────┐
│     (Gateway)        │    │     (Gateway)     │
│ istio-ingressgateway │    │ internal-gateway  │
└─────────┬────────────┘    └─────────┬─────────┘
          │                           │
          └─────────────┬─────────────┘
                        │
                  ┌─────┴───────┐
                  │ (HTTPRoute) │
                  │  toystore   │
                  └─────────────┘
                         ▲
                         │
             ┌───────────┴───────────┐
             │  (RateLimitPolicy)    │
             │ toystore-per-endpoint │
             └───────────────────────┘

① Setup:

make local-setup

kubectl -n kuadrant-system apply -f - <<EOF
apiVersion: kuadrant.io/v1beta1
kind: Kuadrant
metadata:
  name: kuadrant
spec: {}
EOF

kubectl apply -f examples/toystore/toystore.yaml

② Create a second Gateway:

kubectl -n istio-system apply -f - <<EOF
apiVersion: gateway.networking.k8s.io/v1beta1
kind: Gateway
metadata:
  name: internal-gateway
  annotations:
    kuadrant.io/namespace: kuadrant-system
    networking.istio.io/service-type: ClusterIP
spec:
  gatewayClassName: istio
  listeners:
  - name: default
    port: 80
    protocol: HTTP
    hostname: '*.internal'
    allowedRoutes:
      namespaces:
        from: All
EOF

③ Create a HTTPRoute:

(Notice the route specifies 2 gateway parents and 3 host names, where all 3 host names bind the route to the istio-ingressgateway gateway and one of them to the internal-gateway gateway.)

kubectl apply -f - <<EOF
apiVersion: gateway.networking.k8s.io/v1beta1
kind: HTTPRoute
metadata:
  name: toystore
spec:
  parentRefs:
  - name: istio-ingressgateway
    namespace: istio-system
  - name: internal-gateway
    namespace: istio-system
  hostnames:
  - "*.toystore.acme.com"
  - "*.toystore.io"
  - "*.toystore.internal"
  rules:
  - matches:
    - path:
        type: PathPrefix
        value: "/toys"
      method: GET
    - path:
        type: PathPrefix
        value: "/toys"
      method: POST
    backendRefs:
    - name: toystore
      port: 80
  - matches:
    - path:
        type: PathPrefix
        value: "/assets/"
    backendRefs:
    - name: toystore
      port: 80
EOF

④ Create the RateLimitPolicy:

(Notice one of the limits applies to only one host name.)

kubectl apply -f - <<EOF
apiVersion: kuadrant.io/v1beta2
kind: RateLimitPolicy
metadata:
  name: toystore-per-endpoint
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: HTTPRoute
    name: toystore
  limits:
    toys:
      rates:
      - limit: 50
        duration: 1
        unit: minute
      counters:
      - auth.identity.username
      routeSelectors:
      - matches: # matches the 1st HTTPRouteRule (i.e. GET or POST to /toys*)
        - path:
            type: PathPrefix
            value: "/toys"
        hostnames:
        - "*.toystore.acme.com"
      when:
      - selector: auth.identity.group
        operator: neq
        value: admin
    assets:
      rates:
      - limit: 5
        duration: 1
        unit: minute
      - limit: 100
        duration: 12
        unit: hour
      routeSelectors:
      - matches: # matches the 2nd HTTPRouteRule (i.e. /assets/*)
        - path:
            type: PathPrefix
            value: "/assets/"
EOF

⑤ Verify the Limitador CR was reconciled correctly:

kubectl get limitador limitador -n kuadrant-system -o yaml
Expected output
apiVersion: limitador.kuadrant.io/v1alpha1
kind: Limitador
metadata:
  name: limitador
  namespace: kuadrant-system
  
spec:
  limits:
  - conditions:
    - default/toystore-per-endpoint/assets == "1"
    max_value: 5
    namespace: istio-system/istio-ingressgateway#*.toystore.acme.com
    seconds: 60
    variables: []
  - conditions:
    - default/toystore-per-endpoint/assets == "1"
    max_value: 100
    namespace: istio-system/istio-ingressgateway#*.toystore.acme.com
    seconds: 43200
    variables: []
  - conditions:
    - default/toystore-per-endpoint/toys == "1"
    max_value: 50
    namespace: istio-system/istio-ingressgateway#*.toystore.acme.com
    seconds: 60
    variables:
    - auth.identity.username
  - conditions:
    - default/toystore-per-endpoint/assets == "1"
    max_value: 5
    namespace: istio-system/istio-ingressgateway#*.toystore.io
    seconds: 60
    variables: []
  - conditions:
    - default/toystore-per-endpoint/assets == "1"
    max_value: 100
    namespace: istio-system/istio-ingressgateway#*.toystore.io
    seconds: 43200
    variables: []
  - conditions:
    - default/toystore-per-endpoint/toys == "1"
    max_value: 50
    namespace: istio-system/istio-ingressgateway#*.toystore.io
    seconds: 60
    variables:
    - auth.identity.username
  - conditions:
    - default/toystore-per-endpoint/assets == "1"
    max_value: 5
    namespace: istio-system/istio-ingressgateway#*.toystore.internal
    seconds: 60
    variables: []
  - conditions:
    - default/toystore-per-endpoint/assets == "1"
    max_value: 100
    namespace: istio-system/istio-ingressgateway#*.toystore.internal
    seconds: 43200
    variables: []
  - conditions:
    - default/toystore-per-endpoint/toys == "1"
    max_value: 50
    namespace: istio-system/istio-ingressgateway#*.toystore.internal
    seconds: 60
    variables:
    - auth.identity.username
  - conditions:
    - default/toystore-per-endpoint/toys == "1"
    max_value: 50
    namespace: istio-system/internal-gateway#*.toystore.acme.com
    seconds: 60
    variables:
    - auth.identity.username
  - conditions:
    - default/toystore-per-endpoint/assets == "1"
    max_value: 5
    namespace: istio-system/internal-gateway#*.toystore.acme.com
    seconds: 60
    variables: []
  - conditions:
    - default/toystore-per-endpoint/assets == "1"
    max_value: 100
    namespace: istio-system/internal-gateway#*.toystore.acme.com
    seconds: 43200
    variables: []
  - conditions:
    - default/toystore-per-endpoint/toys == "1"
    max_value: 50
    namespace: istio-system/internal-gateway#*.toystore.io
    seconds: 60
    variables:
    - auth.identity.username
  - conditions:
    - default/toystore-per-endpoint/assets == "1"
    max_value: 5
    namespace: istio-system/internal-gateway#*.toystore.io
    seconds: 60
    variables: []
  - conditions:
    - default/toystore-per-endpoint/assets == "1"
    max_value: 100
    namespace: istio-system/internal-gateway#*.toystore.io
    seconds: 43200
    variables: []
  - conditions:
    - default/toystore-per-endpoint/toys == "1"
    max_value: 50
    namespace: istio-system/internal-gateway#*.toystore.internal
    seconds: 60
    variables:
    - auth.identity.username
  - conditions:
    - default/toystore-per-endpoint/assets == "1"
    max_value: 5
    namespace: istio-system/internal-gateway#*.toystore.internal
    seconds: 60
    variables: []
  - conditions:
    - default/toystore-per-endpoint/assets == "1"
    max_value: 100
    namespace: istio-system/internal-gateway#*.toystore.internal
    seconds: 43200
    variables: []
status:
  conditions:
  - lastTransitionTime: "2023-07-06T13:36:32Z"
    message: Limitador is ready
    reason: Ready
    status: "True"
    type: Ready
  observedGeneration: 2
  service:
    host: limitador-limitador.kuadrant-system.svc.cluster.local
    ports:
      grpc: 8081
      http: 8080

⑥ Verify the WasmPlugin CRs were reconciled correctly:

kubectl get wasmplugin kuadrant-istio-ingressgateway -n istio-system  -o yaml
Expected output
apiVersion: extensions.istio.io/v1alpha1
kind: WasmPlugin
metadata:
  name: kuadrant-istio-ingressgateway
  namespace: istio-system
  
spec:
  phase: STATS
  pluginConfig:
    failureMode: deny
    rateLimitPolicies:
    - domain: istio-system/istio-ingressgateway#*.toystore.acme.com
      hostnames:
      - '*.toystore.acme.com'
      - '*.toystore.io'
      - '*.toystore.internal'
      name: default/toystore-per-endpoint
      rules:
      - conditions:
        - allOf:
          - operator: startswith
            selector: request.url_path
            value: /toys
          - operator: eq
            selector: request.method
            value: GET
          - operator: endswith
            selector: request.host
            value: .toystore.acme.com
          - operator: neq
            selector: auth.identity.group
            value: admin
        - allOf:
          - operator: startswith
            selector: request.url_path
            value: /toys
          - operator: eq
            selector: request.method
            value: POST
          - operator: endswith
            selector: request.host
            value: .toystore.acme.com
          - operator: neq
            selector: auth.identity.group
            value: admin
        data:
        - static:
            key: default/toystore-per-endpoint/toys
            value: "1"
        - selector:
            selector: auth.identity.username
      - conditions:
        - allOf:
          - operator: startswith
            selector: request.url_path
            value: /assets/
        data:
        - static:
            key: default/toystore-per-endpoint/assets
            value: "1"
      service: kuadrant-rate-limiting-service
  selector:
    matchLabels:
      app: istio-ingressgateway
      istio: ingressgateway
  url: oci://quay.io/kuadrant/wasm-shim:latest
kubectl get wasmplugin kuadrant-internal-gateway -n istio-system  -o yaml
Expected output
apiVersion: extensions.istio.io/v1alpha1
kind: WasmPlugin
metadata:
  name: kuadrant-internal-gateway
  namespace: istio-system
  
spec:
  phase: STATS
  pluginConfig:
    failureMode: deny
    rateLimitPolicies:
    - domain: istio-system/internal-gateway#*.toystore.internal # <==== this will have to change or it will activate different limits/increase different counters
      hostnames:
      - '*.toystore.internal'
      name: default/toystore-per-endpoint
      rules:
      - conditions:
        - allOf:
          - operator: startswith
            selector: request.url_path
            value: /assets/
        data:
        - static:
            key: default/toystore-per-endpoint/assets
            value: "1"
      service: kuadrant-rate-limiting-service
  selector:
    matchLabels:
      istio.io/gateway-name: internal-gateway
  url: oci://quay.io/kuadrant/wasm-shim:latest

Closes #200
Relates to #156

@guicassolato guicassolato self-assigned this Jun 22, 2023
@guicassolato guicassolato force-pushed the rlpv2-routeselectors branch 6 times, most recently from 3a1d470 to 62a4a43 Compare June 26, 2023 08:51
@guicassolato guicassolato marked this pull request as ready for review July 10, 2023 12:34
@guicassolato guicassolato requested a review from a team as a code owner July 10, 2023 12:34
@@ -94,6 +94,35 @@ func (r *TargetRefReconciler) FetchValidTargetRef(ctx context.Context, targetRef
return nil, fmt.Errorf("FetchValidTargetRef: targetRef (%v) to unknown network resource", targetRef)
}

// FetchAcceptedGatewayHTTPRoutes returns the list of HTTPRoutes that have been accepted as children of a gateway.
func (r *TargetRefReconciler) FetchAcceptedGatewayHTTPRoutes(ctx context.Context, gwKey client.ObjectKey) (routeKeys []gatewayapiv1beta1.HTTPRoute) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that I am asking for anything to happen here or now, but…
Would such functionalities (or the ones in pkg/common/gatewayapi_utils.go) eventually get moved to Kuadrant/gateway-api-machinery ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely yes, @alexsnaps! Not doing it now just because this change goes to the RLPv2 feature branch first and we haven't moved main to using the gateway-api-machinery yet.

@guicassolato
Copy link
Contributor Author

guicassolato commented Jul 10, 2023

Alternative verification steps

Alternative verification steps for a rather complex scenario – with 2 Gateways, 3 HTTPRoutes and 4 RateLimitPolicies.

Intro

Disclaimer

Please keep in mind that we are implementing Direct Policy Attachment and not yet Inherited Policy Attachment. I.e.,

  • no default and overrides;
  • when a policy targets a HTTPRoute, that policy fully replaces any other policies that might exist targeting the HTTPRoute's parents Gateways.

Topology

               *.website
               *.io            *.local
                 │                │
                 ▼                ▼
┌───────┐    ┌──────┐         ┌──────┐     ┌───────┐
│ rlp-1 ├───►│ gw-1 │         │ gw-2 │◄────┤ rlp-2 │
└───────┘    └─┬──┬─┘         └─┬──┬─┘     └───────┘
               │  │             │  │
         ┌─────┘  └──────┬──────┘  └────┐
         │               │              │
    ┌────┴────┐     ┌────┴────┐    ┌────┴────┐
    │ route-1 │     │ route-2 │    │ route-3 │
    └─────────┘     └─────────┘    └─────────┘
         ▲               ▲
         │               │
     ┌───┴───┐       ┌───┴───┐
     │ rlp-3 │       │ rlp-4 │
     └───────┘       └───────┘

Description of the network resources

  • gw-1: N/S (internet) traffic gateway; declares 2 listeners: *.website (for public websites) and *.io (for APIs exposed to the internet)
  • gw-2: E/W (local) traffic gateway; declares 1 listener – *.local (for APIs consumed within the network)
  • route-1: attached to the N/S (internet) gateway only; catches traffic directed to *.toystore.(website|io); declares 2 route rules: GET /v1/* or POST /v1/*, and /assets/*
  • route-2: attached to both gateways; catches traffic directed to status.(io|local); declares 1 route rule: /* ("catch-all")
  • route-3: attached to the E/W local gateway only; catches the complement of the traffic directed to *.local (i.e. all except status.local); declares 2 route rules: /v1/* and /v2/*

State transitions

After setting the topology as described above, each RLP (from 1 to 4) will be created in sequence. This will trigger (4x) the reconciliation of the WasmPlugin CRs named kuadrant-gw-1 and kuadrant-gw-2, respectivelly for each gateway.

A WasmPlugin contains the configs for the RLPs and corresponding affected HTTPRoutes, according to the targetRefs of each RLP. For example, when rlp-1 is created, because it affects two HTTPRoutes (i.e. route-1 and route-2, by declaring gw-1 as parent), a WasmPlugin CR will be created for this gateway, with name kuadrant-gw-1, and containing instructions to activate the limits declared in rlp-1 for the route rules stated by route-1 and route-2. Later, when rlp-3 is created, it replaces rlp-1 as the desired state for protecting route-1; nevertheless, rlp-1 remains valid for any traffic that flows through gw-1 to route-2, until rlp-4 is created.

Similarly, and for all the other RLPs and affected HTTPRoutes, the table below describes the expected states of the WasmPluging CRs after the creation of each policy.

State After
steps
WasmPlugins
reconciled
Config contains
limits from RLPs
Activated on
matches from
0 ① ② ③ N/A N/A
1 ④ rlp-1 kuadrant-gw-1 rlp-1 route-1 | route-2
2 ⑤ rlp-2 kuadrant-gw-2 rlp-2 route-2 | route-3
3 ⑥ rlp-3 kuadrant-gw-1 rlp-1 route-2
rlp-3 route-1
kuadrant-gw-2 rlp-2 route-2 | route-3
4 ⑦ rlp-4 kuadrant-gw-1 rlp-1
rlp-3 route-1
rlp-4 route-2
kuadrant-gw-2 rlp-2 route-3
rlp-4 route-2

Expected final state

After the creation of all RLPs, any additional event that does not change the topology reconciles the following final state:

WasmPlugin Limits from Activated on
kuadrant-gw-1 rlp-1
rlp-3 route-1
rlp-4 route-2
kuadrant-gw-2 rlp-2 route-3
rlp-4 route-2

At the final state, the following back reference annotations are expected to be found in each of the targeted network resources:

Gateway Annotation RLPs
gw-1 kuadrant.io/ratelimitpolicies rlp-1, rlp-3, rlp-4
kuadrant.io/ratelimitpolicy rlp-1
gw-2 kuadrant.io/ratelimitpolicies rlp-2, rlp-4
kuadrant.io/ratelimitpolicy rlp-2
route-1 kuadrant.io/ratelimitpolicy rlp-3
route-2 kuadrant.io/ratelimitpolicy rlp-4
route-3

Steps ① → ⑦

① Setup

make local-setup

kubectl -n kuadrant-system apply -f - <<EOF
apiVersion: kuadrant.io/v1beta1
kind: Kuadrant
metadata:
  name: kuadrant
spec: {}
EOF

kubectl apply -f examples/toystore/toystore.yaml

② Create the Gateways

kubectl -n istio-system apply -f - <<EOF
apiVersion: gateway.networking.k8s.io/v1beta1
kind: Gateway
metadata:
  name: gw-1
  annotations:
    kuadrant.io/namespace: kuadrant-system
    networking.istio.io/service-type: ClusterIP
spec:
  gatewayClassName: istio
  listeners:
  - name: websites
    port: 80
    protocol: HTTP
    hostname: '*.website'
    allowedRoutes:
      namespaces:
        from: All
  - name: apis
    port: 80
    protocol: HTTP
    hostname: '*.io'
    allowedRoutes:
      namespaces:
        from: All
---
apiVersion: gateway.networking.k8s.io/v1beta1
kind: Gateway
metadata:
  name: gw-2
  annotations:
    kuadrant.io/namespace: kuadrant-system
    networking.istio.io/service-type: ClusterIP
spec:
  gatewayClassName: istio
  listeners:
  - name: local
    port: 80
    protocol: HTTP
    hostname: '*.local'
    allowedRoutes:
      namespaces:
        from: All
EOF

③ Create the HTTPRoutes

kubectl apply -f - <<EOF
apiVersion: gateway.networking.k8s.io/v1beta1
kind: HTTPRoute
metadata:
  name: route-1
spec:
  parentRefs:
  - namespace: istio-system
    name: gw-1
  hostnames:
  - "*.toystore.website"
  - "*.toystore.io"
  rules:
  - matches:
    - method: GET
      path: { value: "/v1/", type: PathPrefix }
    - method: POST
      path: { value: "/v1/", type: PathPrefix }
    backendRefs:
    - name: toystore
      port: 80
  - matches:
    - path: { value: "/assets/", type: PathPrefix }
    backendRefs:
    - name: toystore
      port: 80
---
apiVersion: gateway.networking.k8s.io/v1beta1
kind: HTTPRoute
metadata:
  name: route-2
spec:
  parentRefs:
  - namespace: istio-system
    name: gw-1
  - namespace: istio-system
    name: gw-2
  hostnames:
  - status.io
  - status.local
  rules:
  - backendRefs:
    - name: toystore
      port: 80
---
apiVersion: gateway.networking.k8s.io/v1beta1
kind: HTTPRoute
metadata:
  name: route-3
spec:
  parentRefs:
  - namespace: istio-system
    name: gw-2
  rules:
  - matches:
    - path: { value: "/v1/", type: PathPrefix }
    - path: { value: "/v2/", type: PathPrefix }
    backendRefs:
    - name: toystore
      port: 80
EOF

Check no WasmPlugins created at this stage:

kubectl get wasmplugins -A
# No resources found

④ Create rlp-1 targetting gw-1

kubectl -n istio-system apply -f - <<EOF
apiVersion: kuadrant.io/v1beta2
kind: RateLimitPolicy
metadata:
  name: rlp-1
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: Gateway
    name: gw-1
  limits:
    internet-traffic-all:
      rates:
      - limit: 1000
        duration: 1
        unit: second
    internet-traffic-apis-per-host:
      rates:
      - limit: 200
        duration: 1
        unit: second
      counters:
      - request.host
      when:
      - selector: request.host
        operator: endswith
        value: .io
EOF

Check the WasmPlugin for gateway gw-1:

kubectl get wasmplugin/kuadrant-gw-1 -n istio-system -o yaml
Expected output
apiVersion: extensions.istio.io/v1alpha1
kind: WasmPlugin
metadata:
  name: kuadrant-gw-1
  namespace: istio-system
  
spec:
  phase: STATS
  pluginConfig:
    failureMode: deny
    rateLimitPolicies:
    - domain: istio-system/gw-1#*.website
      hostnames:
      - '*.website'
      - '*.io'
      name: istio-system/rlp-1
      rules:
      - conditions:
        - allOf: # from route-1
          - operator: startswith
            selector: request.url_path
            value: /v1/
          - operator: eq
            selector: request.method
            value: GET
        - allOf: # from route-1
          - operator: startswith
            selector: request.url_path
            value: /v1/
          - operator: eq
            selector: request.method
            value: POST
        - allOf: # from route-1
          - operator: startswith
            selector: request.url_path
            value: /assets/
        - allOf: # from route-2
          - operator: startswith
            selector: request.url_path
            value: /
        data:
        - static:
            key: istio-system/rlp-1/internet-traffic-all
            value: "1"
      - conditions:
        - allOf: # from route-1
          - operator: startswith
            selector: request.url_path
            value: /v1/
          - operator: eq
            selector: request.method
            value: GET
          - operator: endswith
            selector: request.host
            value: .io
        - allOf: # from route-1
          - operator: startswith
            selector: request.url_path
            value: /v1/
          - operator: eq
            selector: request.method
            value: POST
          - operator: endswith
            selector: request.host
            value: .io
        - allOf: # from route-1
          - operator: startswith
            selector: request.url_path
            value: /assets/
          - operator: endswith
            selector: request.host
            value: .io
        - allOf: # from route-2
          - operator: startswith
            selector: request.url_path
            value: /
          - operator: endswith
            selector: request.host
            value: .io
        data:
        - static:
            key: istio-system/rlp-1/internet-traffic-apis-per-host
            value: "1"
        - selector:
            selector: request.host
      service: kuadrant-rate-limiting-service
  selector:
    matchLabels:
      istio.io/gateway-name: gw-1
  url: oci://quay.io/kuadrant/wasm-shim:latest

No WasmPlugin for gateway gw-2 should exist at this point.

⑤ Create rlp-2 targetting gw-2

kubectl -n istio-system apply -f - <<EOF
apiVersion: kuadrant.io/v1beta2
kind: RateLimitPolicy
metadata:
  name: rlp-2
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: Gateway
    name: gw-2
  limits:
    local-traffic-non-admins-per-host:
      rates:
      - limit: 500
        duration: 1
        unit: second
      counters:
      - request.host
      when:
      - selector: auth.identity.groups
        operator: excl
        value: admins
EOF

Check the WasmPlugin for gateway gw-2:

kubectl get wasmplugin/kuadrant-gw-2 -n istio-system -o yaml
Expected output
apiVersion: extensions.istio.io/v1alpha1
kind: WasmPlugin
metadata:
  name: kuadrant-gw-2
  namespace: istio-system
  
spec:
  phase: STATS
  pluginConfig:
    failureMode: deny
    rateLimitPolicies:
    - domain: istio-system/gw-2#*.local
      hostnames:
      - '*.local'
      name: istio-system/rlp-2
      rules:
      - conditions:
        - allOf: # from route-2
          - operator: startswith
            selector: request.url_path
            value: /
          - operator: excl
            selector: auth.identity.groups
            value: admins
        - allOf: # from route-3
          - operator: startswith
            selector: request.url_path
            value: /v1/
          - operator: excl
            selector: auth.identity.groups
            value: admins
        - allOf: # from route-3
          - operator: startswith
            selector: request.url_path
            value: /v2/
          - operator: excl
            selector: auth.identity.groups
            value: admins
        data:
        - static:
            key: istio-system/rlp-2/local-traffic-non-admins-per-host
            value: "1"
        - selector:
            selector: request.host
      service: kuadrant-rate-limiting-service
  selector:
    matchLabels:
      istio.io/gateway-name: gw-2
  url: oci://quay.io/kuadrant/wasm-shim:latest

The WasmPlugin for gateway gw-1 should be unchanged.

⑥ Create rlp-3 targetting route-1

kubectl apply -f - <<EOF
apiVersion: kuadrant.io/v1beta2
kind: RateLimitPolicy
metadata:
  name: rlp-3
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: HTTPRoute
    name: route-1
  limits:
    toystore-v1-website-unauthenticated:
      rates:
      - limit: 50
        duration: 1
        unit: minute
      routeSelectors:
      - matches: # matches the 1st HTTPRouteRule (GET|POST /v1/*)
        - path:
            type: PathPrefix
            value: "/v1/"
        hostnames:
        - "*.toystore.website"
      when:
      - selector: auth.identity.username
        operator: eq
        value: ""
    toystore-assets-all-domains:
      rates:
      - limit: 5
        duration: 1
        unit: minute
      - limit: 100
        duration: 12
        unit: hour
      routeSelectors:
      - matches: # matches the 2nd HTTPRouteRule (/assets/*)
        - path:
            type: PathPrefix
            value: "/assets/"
EOF

Check the WasmPlugin for gateway gw-1:

kubectl get wasmplugin/kuadrant-gw-1 -n istio-system -o yaml
Expected output
apiVersion: extensions.istio.io/v1alpha1
kind: WasmPlugin
metadata:
  name: kuadrant-gw-1
  namespace: istio-system
  
spec:
  phase: STATS
  pluginConfig:
    failureMode: deny
    rateLimitPolicies:
    - domain: istio-system/gw-1#*.website
      hostnames:
      - '*.website'
      - '*.io'
      name: istio-system/rlp-1
      rules:
      - conditions:
        - allOf: # from route-2
          - operator: startswith
            selector: request.url_path
            value: /
        data:
        - static:
            key: istio-system/rlp-1/internet-traffic-all
            value: "1"
      - conditions:
        - allOf: # from route-2
          - operator: startswith
            selector: request.url_path
            value: /
          - operator: endswith
            selector: request.host
            value: .io
        data:
        - static:
            key: istio-system/rlp-1/internet-traffic-apis-per-host
            value: "1"
        - selector:
            selector: request.host
      service: kuadrant-rate-limiting-service
    - domain: istio-system/gw-1#*.toystore.website
      hostnames:
      - '*.toystore.website'
      - '*.toystore.io'
      name: default/rlp-3
      rules:
      - conditions:
        - allOf: # from route-1
          - operator: startswith
            selector: request.url_path
            value: /assets/
        data:
        - static:
            key: default/rlp-3/toystore-assets-all-domains
            value: "1"
      - conditions:
        - allOf: # from route-1
          - operator: startswith
            selector: request.url_path
            value: /v1/
          - operator: eq
            selector: request.method
            value: GET
          - operator: endswith
            selector: request.host
            value: .toystore.website
          - operator: eq
            selector: auth.identity.username
            value: ""
        - allOf: # from route-1
          - operator: startswith
            selector: request.url_path
            value: /v1/
          - operator: eq
            selector: request.method
            value: POST
          - operator: endswith
            selector: request.host
            value: .toystore.website
          - operator: eq
            selector: auth.identity.username
            value: ""
        data:
        - static:
            key: default/rlp-3/toystore-v1-website-unauthenticated
            value: "1"
      service: kuadrant-rate-limiting-service
  selector:
    matchLabels:
      istio.io/gateway-name: gw-1
  url: oci://quay.io/kuadrant/wasm-shim:latest

The WasmPlugin for gateway gw-2 should be unchanged.

⑦ Create rlp-4 targetting route-2

kubectl apply -f - <<EOF
apiVersion: kuadrant.io/v1beta2
kind: RateLimitPolicy
metadata:
  name: rlp-4
spec:
  targetRef:
    group: gateway.networking.k8s.io
    kind: HTTPRoute
    name: route-2
  limits:
    status-per-ip:
      rates:
      - limit: 10
        duration: 5
        unit: minute
      counters:
      - source.address
EOF

Check the WasmPlugins for both gateways:

kubectl get wasmplugin/kuadrant-gw-1 -n istio-system -o yaml
Expected output
apiVersion: extensions.istio.io/v1alpha1
kind: WasmPlugin
metadata:
  name: kuadrant-gw-1
  namespace: istio-system
  
spec:
  phase: STATS
  pluginConfig:
    failureMode: deny
    rateLimitPolicies:
    - domain: istio-system/gw-1#*.toystore.website
      hostnames:
      - '*.toystore.website'
      - '*.toystore.io'
      name: default/rlp-3
      rules:
      - conditions:
        - allOf: # from route-1
          - operator: startswith
            selector: request.url_path
            value: /assets/
        data:
        - static:
            key: default/rlp-3/toystore-assets-all-domains
            value: "1"
      - conditions:
        - allOf: # from route-1
          - operator: startswith
            selector: request.url_path
            value: /v1/
          - operator: eq
            selector: request.method
            value: GET
          - operator: endswith
            selector: request.host
            value: .toystore.website
          - operator: eq
            selector: auth.identity.username
            value: ""
        - allOf: # from route-1
          - operator: startswith
            selector: request.url_path
            value: /v1/
          - operator: eq
            selector: request.method
            value: POST
          - operator: endswith
            selector: request.host
            value: .toystore.website
          - operator: eq
            selector: auth.identity.username
            value: ""
        data:
        - static:
            key: default/rlp-3/toystore-v1-website-unauthenticated
            value: "1"
      service: kuadrant-rate-limiting-service
    - domain: istio-system/gw-1#status.io
      hostnames:
      - status.io
      name: default/rlp-4
      rules:
      - conditions:
        - allOf: # from route-2
          - operator: startswith
            selector: request.url_path
            value: /
        data:
        - static:
            key: default/rlp-4/status-per-ip
            value: "1"
        - selector:
            selector: source.address
      service: kuadrant-rate-limiting-service
  selector:
    matchLabels:
      istio.io/gateway-name: gw-1
  url: oci://quay.io/kuadrant/wasm-shim:latest
kubectl get wasmplugin/kuadrant-gw-2 -n istio-system -o yaml
Expected output
apiVersion: extensions.istio.io/v1alpha1
kind: WasmPlugin
metadata:
  name: kuadrant-gw-2
  namespace: istio-system
  
spec:
  phase: STATS
  pluginConfig:
    failureMode: deny
    rateLimitPolicies:
    - domain: istio-system/gw-2#*.local
      hostnames:
      - '*.local'
      name: istio-system/rlp-2
      rules:
      - conditions:
        - allOf: # from route-3
          - operator: startswith
            selector: request.url_path
            value: /v1/
          - operator: excl
            selector: auth.identity.groups
            value: admins
        - allOf: # from route-3
          - operator: startswith
            selector: request.url_path
            value: /v2/
          - operator: excl
            selector: auth.identity.groups
            value: admins
        data:
        - static:
            key: istio-system/rlp-2/local-traffic-non-admins-per-host
            value: "1"
        - selector:
            selector: request.host
      service: kuadrant-rate-limiting-service
    - domain: istio-system/gw-2#status.local
      hostnames:
      - status.local
      name: default/rlp-4
      rules:
      - conditions:
        - allOf: # from route-2
          - operator: startswith
            selector: request.url_path
            value: /
        data:
        - static:
            key: default/rlp-4/status-per-ip
            value: "1"
        - selector:
            selector: source.address
      service: kuadrant-rate-limiting-service
  selector:
    matchLabels:
      istio.io/gateway-name: gw-2
  url: oci://quay.io/kuadrant/wasm-shim:latest

pkg/common/common.go Outdated Show resolved Hide resolved
@guicassolato guicassolato requested a review from a team July 11, 2023 09:36
@guicassolato guicassolato marked this pull request as draft July 11, 2023 16:09
@guicassolato guicassolato marked this pull request as ready for review July 11, 2023 18:28
@guicassolato guicassolato force-pushed the rlpv2-routeselectors branch 3 times, most recently from 185f46f to 61edeea Compare July 12, 2023 08:27
@guicassolato guicassolato changed the title [rlp-v2] reconciliations of route selectors from httprouterules [rlp-v2] WasmPlugin reconciliation Jul 12, 2023
* Change `common.Contains(slice []string, target string)` to any comparable type (using generics)
* Define a `common.HTTPRouteRuleSelector` type that checks whether a `HTTPRouteMatch` selects a `HTTPRouteRule` or not
* Define a `rlptools.HTTPRouteRulesFromRouteSelector` function that returns all rules from a HTTPRoute that a route selector selects (using `common.HTTPRouteRuleSelector`)
* Modify `rlptools.conditionsFromLimit` to use `rlptools.HTTPRouteRulesFromRouteSelector` and generate wasm conditions for each combination of selected route rule match and hostname
* Ensure all generated route conditions include a pattern for the hostnames (route selectors’, HTTPRoute’s or Gateway’s)
* Ensure all generated route conditions include the user-defined `when` conditions
…te and complex ratelimitpolicy

The httproute declares 2 rules and 2 hostnames.

The RLP declares 2 limits:
- one limit declares one counter, targets one of the httprouterules, for one hostname only, with an additional 'when' condition and counter qualifier;
- the other limit declares 2 counters, targets the other httprouterule, for all hostnames, and states no additional 'when' condition nor counter qualifier.
@guicassolato guicassolato force-pushed the rlpv2-routeselectors branch from 87bf616 to 0bbadd2 Compare July 14, 2023 09:44
@@ -20,7 +21,7 @@ var (
WASMFilterImageURL = common.FetchEnv("RELATED_IMAGE_WASMSHIM", "oci://quay.io/kuadrant/wasm-shim:latest")
)

// WasmRules computes WASM rules from the policy and the targeted Route (which can be nil when a gateway is targeted)
// WasmRules computes WASM rules from the policy and the targeted route.
func WasmRules(rlp *kuadrantv1beta2.RateLimitPolicy, route *gatewayapiv1beta1.HTTPRoute) []wasm.Rule {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method should return error as well. One of the use cases I found is when the route selector does not match any HTTPRoute rule (route)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an alternative, the route selectors validity could be evaluated in a early stage of the reconcilliation loop to report in the status when the selector does not select any route. With this approach, the WasmRules only returns rules

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now, it's simply skipping a route selector that does not select anything.

Instead of raising a error here, I'd advocate for a combination of the following:

  1. the alternative you suggest – i.e. checking for route selector mismatches early on; I'd say in a validating webhook;
  2. reporting in the status of the RLP about all limits that will not be applied due to route selector mismatch. This one because this reconciliation can be triggered by changes in the HTTPRoutes as well.

@eguzki
Copy link
Contributor

eguzki commented Jul 17, 2023

verification steps working

@eguzki
Copy link
Contributor

eguzki commented Jul 17, 2023

Tested removing httproutes and rlp's to see how reconciliation handles and works as expected 💪

@eguzki
Copy link
Contributor

eguzki commented Jul 17, 2023

I think now I (fully) understand the route selector concept. We really need to document it clearly. If routeSelector are thought as traffic qualifiers for rate limiting, that may lead to unexpected behavior. routeSelectors are (surprise) nothing else than a HTTPRoute rule (route) selector. And the traffic qualifiers for rate limiting are the (selected) HTTPRoute rules.

Example:

  • HTTPRoute rules: GET /v1* or POST /v1*
  • RLP routeSelector: GET /v1*

The traffic to be rate limited will be: GET /v1* or POST /v1*. One (like me) might expect to be GET /v1*

@didierofrivia
Copy link
Member

Verification steps working, both guides!

BTW, super useful to extract to a user guide 📖

Passes a list of hostnames into the call to the function that generates the wasm rules for a given RLP, notoriously to compute the conditions when to apply each limit. The list of hostnames is the domain intersection between the hostnames specified in the HTTPRoute and its parent gateway for which the wasm config is being built. The route selection considers this as a boundary, so it doesn’t generate rules for the wrong gateway/hostname.

Fixes the computation of conditions when no HTTPRouteRule matches the route selectors.
…e_selectors.go` along with the `RouteSelector` type itself

+ no longer injecting a separate list of hostnames for avoiding building wasm rules for hostnames that do not apply, as this did not seem to belong to the route selector logic anyway.
Get all HTTPRoutes that are children of the Gateway and call the generation of the set of wasm rules (conditions and data for each limit) for each HTTPRoute
…ng the wasm rules for rlps that target a gateway
…TTPRouteRules in the same order as the selectors select the rules
…tes that have a rlp of its own and avoid building wasmplugins when there are no rules to apply
@guicassolato guicassolato force-pushed the rlpv2-routeselectors branch from 0bbadd2 to 98cbe4a Compare July 17, 2023 17:59
@guicassolato guicassolato requested a review from eguzki July 18, 2023 15:43
@guicassolato guicassolato merged commit 529ae66 into rlp-v2-base Jul 20, 2023
@guicassolato guicassolato deleted the rlpv2-routeselectors branch July 20, 2023 09:39
KevFan pushed a commit to KevFan/kuadrant-operator that referenced this pull request Aug 10, 2023
* reconciliation of route selectors for RLPs targeting HTTPRoutes

* Change `common.Contains(slice []string, target string)` to any comparable type (using generics)
* Define a `common.HTTPRouteRuleSelector` type that checks whether a `HTTPRouteMatch` selects a `HTTPRouteRule` or not
* Define a `rlptools.HTTPRouteRulesFromRouteSelector` function that returns all rules from a HTTPRoute that a route selector selects (using `common.HTTPRouteRuleSelector`)
* Modify `rlptools.conditionsFromLimit` to use `rlptools.HTTPRouteRulesFromRouteSelector` and generate wasm conditions for each combination of selected route rule match and hostname
* Ensure all generated route conditions include a pattern for the hostnames (route selectors’, HTTPRoute’s or Gateway’s)
* Ensure all generated route conditions include the user-defined `when` conditions

* add tests for generating a wasmplugin config out of a complex httproute and complex ratelimitpolicy

The httproute declares 2 rules and 2 hostnames.

The RLP declares 2 limits:
- one limit declares one counter, targets one of the httprouterules, for one hostname only, with an additional 'when' condition and counter qualifier;
- the other limit declares 2 counters, targets the other httprouterule, for all hostnames, and states no additional 'when' condition nor counter qualifier.

* generate wasm rules only for routeSelector-filtered hostnames that belong to the targeted route

* fix: pattern expression for hostnames with the right operator - introduces 'endswith' pattern operator

* fix: compute wasm rules only for the applicable hostnames

Passes a list of hostnames into the call to the function that generates the wasm rules for a given RLP, notoriously to compute the conditions when to apply each limit. The list of hostnames is the domain intersection between the hostnames specified in the HTTPRoute and its parent gateway for which the wasm config is being built. The route selection considers this as a boundary, so it doesn’t generate rules for the wrong gateway/hostname.

Fixes the computation of conditions when no HTTPRouteRule matches the route selectors.

* do not generate wasm condition patterns for the hostname when all apply

* do not repeat RLPs for each hostname within the wasm config

* `rlptools.HTTPRouteRulesFromRouteSelector` moved to `api/v1beta2/route_selectors.go` along with the `RouteSelector` type itself

+ no longer injecting a separate list of hostnames for avoiding building wasm rules for hostnames that do not apply, as this did not seem to belong to the route selector logic anyway.

* reconciliation of route selectors for RLPs targeting Gateways

Get all HTTPRoutes that are children of the Gateway and call the generation of the set of wasm rules (conditions and data for each limit) for each HTTPRoute

* match the gateway hostnames instead of the httproute ones when building the wasm rules for rlps that target a gateway

* refactor: make HTTPRouteRuleSelector.Selects return sooner if the HTTP method does not match

* refactor: ensure wasm rule conditions are generated out of selected HTTPRouteRules in the same order as the selectors select the rules

* Prevent the usage of routeSelectors in RLPs that target a Gateway

* do not generate conditions for gateway policy wasm rules from httproutes that have a rlp of its own and avoid building wasmplugins when there are no rules to apply

* avoid adding rlps without any rules to the wasm config
KevFan pushed a commit to KevFan/kuadrant-operator that referenced this pull request Aug 14, 2023
* reconciliation of route selectors for RLPs targeting HTTPRoutes

* Change `common.Contains(slice []string, target string)` to any comparable type (using generics)
* Define a `common.HTTPRouteRuleSelector` type that checks whether a `HTTPRouteMatch` selects a `HTTPRouteRule` or not
* Define a `rlptools.HTTPRouteRulesFromRouteSelector` function that returns all rules from a HTTPRoute that a route selector selects (using `common.HTTPRouteRuleSelector`)
* Modify `rlptools.conditionsFromLimit` to use `rlptools.HTTPRouteRulesFromRouteSelector` and generate wasm conditions for each combination of selected route rule match and hostname
* Ensure all generated route conditions include a pattern for the hostnames (route selectors’, HTTPRoute’s or Gateway’s)
* Ensure all generated route conditions include the user-defined `when` conditions

* add tests for generating a wasmplugin config out of a complex httproute and complex ratelimitpolicy

The httproute declares 2 rules and 2 hostnames.

The RLP declares 2 limits:
- one limit declares one counter, targets one of the httprouterules, for one hostname only, with an additional 'when' condition and counter qualifier;
- the other limit declares 2 counters, targets the other httprouterule, for all hostnames, and states no additional 'when' condition nor counter qualifier.

* generate wasm rules only for routeSelector-filtered hostnames that belong to the targeted route

* fix: pattern expression for hostnames with the right operator - introduces 'endswith' pattern operator

* fix: compute wasm rules only for the applicable hostnames

Passes a list of hostnames into the call to the function that generates the wasm rules for a given RLP, notoriously to compute the conditions when to apply each limit. The list of hostnames is the domain intersection between the hostnames specified in the HTTPRoute and its parent gateway for which the wasm config is being built. The route selection considers this as a boundary, so it doesn’t generate rules for the wrong gateway/hostname.

Fixes the computation of conditions when no HTTPRouteRule matches the route selectors.

* do not generate wasm condition patterns for the hostname when all apply

* do not repeat RLPs for each hostname within the wasm config

* `rlptools.HTTPRouteRulesFromRouteSelector` moved to `api/v1beta2/route_selectors.go` along with the `RouteSelector` type itself

+ no longer injecting a separate list of hostnames for avoiding building wasm rules for hostnames that do not apply, as this did not seem to belong to the route selector logic anyway.

* reconciliation of route selectors for RLPs targeting Gateways

Get all HTTPRoutes that are children of the Gateway and call the generation of the set of wasm rules (conditions and data for each limit) for each HTTPRoute

* match the gateway hostnames instead of the httproute ones when building the wasm rules for rlps that target a gateway

* refactor: make HTTPRouteRuleSelector.Selects return sooner if the HTTP method does not match

* refactor: ensure wasm rule conditions are generated out of selected HTTPRouteRules in the same order as the selectors select the rules

* Prevent the usage of routeSelectors in RLPs that target a Gateway

* do not generate conditions for gateway policy wasm rules from httproutes that have a rlp of its own and avoid building wasmplugins when there are no rules to apply

* avoid adding rlps without any rules to the wasm config
KevFan pushed a commit to KevFan/kuadrant-operator that referenced this pull request Aug 14, 2023
* reconciliation of route selectors for RLPs targeting HTTPRoutes

* Change `common.Contains(slice []string, target string)` to any comparable type (using generics)
* Define a `common.HTTPRouteRuleSelector` type that checks whether a `HTTPRouteMatch` selects a `HTTPRouteRule` or not
* Define a `rlptools.HTTPRouteRulesFromRouteSelector` function that returns all rules from a HTTPRoute that a route selector selects (using `common.HTTPRouteRuleSelector`)
* Modify `rlptools.conditionsFromLimit` to use `rlptools.HTTPRouteRulesFromRouteSelector` and generate wasm conditions for each combination of selected route rule match and hostname
* Ensure all generated route conditions include a pattern for the hostnames (route selectors’, HTTPRoute’s or Gateway’s)
* Ensure all generated route conditions include the user-defined `when` conditions

* add tests for generating a wasmplugin config out of a complex httproute and complex ratelimitpolicy

The httproute declares 2 rules and 2 hostnames.

The RLP declares 2 limits:
- one limit declares one counter, targets one of the httprouterules, for one hostname only, with an additional 'when' condition and counter qualifier;
- the other limit declares 2 counters, targets the other httprouterule, for all hostnames, and states no additional 'when' condition nor counter qualifier.

* generate wasm rules only for routeSelector-filtered hostnames that belong to the targeted route

* fix: pattern expression for hostnames with the right operator - introduces 'endswith' pattern operator

* fix: compute wasm rules only for the applicable hostnames

Passes a list of hostnames into the call to the function that generates the wasm rules for a given RLP, notoriously to compute the conditions when to apply each limit. The list of hostnames is the domain intersection between the hostnames specified in the HTTPRoute and its parent gateway for which the wasm config is being built. The route selection considers this as a boundary, so it doesn’t generate rules for the wrong gateway/hostname.

Fixes the computation of conditions when no HTTPRouteRule matches the route selectors.

* do not generate wasm condition patterns for the hostname when all apply

* do not repeat RLPs for each hostname within the wasm config

* `rlptools.HTTPRouteRulesFromRouteSelector` moved to `api/v1beta2/route_selectors.go` along with the `RouteSelector` type itself

+ no longer injecting a separate list of hostnames for avoiding building wasm rules for hostnames that do not apply, as this did not seem to belong to the route selector logic anyway.

* reconciliation of route selectors for RLPs targeting Gateways

Get all HTTPRoutes that are children of the Gateway and call the generation of the set of wasm rules (conditions and data for each limit) for each HTTPRoute

* match the gateway hostnames instead of the httproute ones when building the wasm rules for rlps that target a gateway

* refactor: make HTTPRouteRuleSelector.Selects return sooner if the HTTP method does not match

* refactor: ensure wasm rule conditions are generated out of selected HTTPRouteRules in the same order as the selectors select the rules

* Prevent the usage of routeSelectors in RLPs that target a Gateway

* do not generate conditions for gateway policy wasm rules from httproutes that have a rlp of its own and avoid building wasmplugins when there are no rules to apply

* avoid adding rlps without any rules to the wasm config
KevFan pushed a commit to KevFan/kuadrant-operator that referenced this pull request Aug 16, 2023
* reconciliation of route selectors for RLPs targeting HTTPRoutes

* Change `common.Contains(slice []string, target string)` to any comparable type (using generics)
* Define a `common.HTTPRouteRuleSelector` type that checks whether a `HTTPRouteMatch` selects a `HTTPRouteRule` or not
* Define a `rlptools.HTTPRouteRulesFromRouteSelector` function that returns all rules from a HTTPRoute that a route selector selects (using `common.HTTPRouteRuleSelector`)
* Modify `rlptools.conditionsFromLimit` to use `rlptools.HTTPRouteRulesFromRouteSelector` and generate wasm conditions for each combination of selected route rule match and hostname
* Ensure all generated route conditions include a pattern for the hostnames (route selectors’, HTTPRoute’s or Gateway’s)
* Ensure all generated route conditions include the user-defined `when` conditions

* add tests for generating a wasmplugin config out of a complex httproute and complex ratelimitpolicy

The httproute declares 2 rules and 2 hostnames.

The RLP declares 2 limits:
- one limit declares one counter, targets one of the httprouterules, for one hostname only, with an additional 'when' condition and counter qualifier;
- the other limit declares 2 counters, targets the other httprouterule, for all hostnames, and states no additional 'when' condition nor counter qualifier.

* generate wasm rules only for routeSelector-filtered hostnames that belong to the targeted route

* fix: pattern expression for hostnames with the right operator - introduces 'endswith' pattern operator

* fix: compute wasm rules only for the applicable hostnames

Passes a list of hostnames into the call to the function that generates the wasm rules for a given RLP, notoriously to compute the conditions when to apply each limit. The list of hostnames is the domain intersection between the hostnames specified in the HTTPRoute and its parent gateway for which the wasm config is being built. The route selection considers this as a boundary, so it doesn’t generate rules for the wrong gateway/hostname.

Fixes the computation of conditions when no HTTPRouteRule matches the route selectors.

* do not generate wasm condition patterns for the hostname when all apply

* do not repeat RLPs for each hostname within the wasm config

* `rlptools.HTTPRouteRulesFromRouteSelector` moved to `api/v1beta2/route_selectors.go` along with the `RouteSelector` type itself

+ no longer injecting a separate list of hostnames for avoiding building wasm rules for hostnames that do not apply, as this did not seem to belong to the route selector logic anyway.

* reconciliation of route selectors for RLPs targeting Gateways

Get all HTTPRoutes that are children of the Gateway and call the generation of the set of wasm rules (conditions and data for each limit) for each HTTPRoute

* match the gateway hostnames instead of the httproute ones when building the wasm rules for rlps that target a gateway

* refactor: make HTTPRouteRuleSelector.Selects return sooner if the HTTP method does not match

* refactor: ensure wasm rule conditions are generated out of selected HTTPRouteRules in the same order as the selectors select the rules

* Prevent the usage of routeSelectors in RLPs that target a Gateway

* do not generate conditions for gateway policy wasm rules from httproutes that have a rlp of its own and avoid building wasmplugins when there are no rules to apply

* avoid adding rlps without any rules to the wasm config
guicassolato added a commit that referenced this pull request Aug 21, 2023
* rpl v2 CRD

* RLP CRD v2: WhenConditionOperator only eq and neq

* controller-gen paths for v1beta2 as well

* minor enhancement about logging only in debug mode

* rlp v1beta2 controller

* WASM plugin configuration object v2

* bundle update

* fixup! bundle update

* fix tests

* WASMPlugin update to camel case

* fix integration tests

* wasm types moved to a common package

* different selectors share type: ContextSelector

---------

Co-authored-by: Eguzki Astiz Lezaun <[email protected]>

* [rlp-v2] WasmPlugin reconciliation  (#204)

* reconciliation of route selectors for RLPs targeting HTTPRoutes

* Change `common.Contains(slice []string, target string)` to any comparable type (using generics)
* Define a `common.HTTPRouteRuleSelector` type that checks whether a `HTTPRouteMatch` selects a `HTTPRouteRule` or not
* Define a `rlptools.HTTPRouteRulesFromRouteSelector` function that returns all rules from a HTTPRoute that a route selector selects (using `common.HTTPRouteRuleSelector`)
* Modify `rlptools.conditionsFromLimit` to use `rlptools.HTTPRouteRulesFromRouteSelector` and generate wasm conditions for each combination of selected route rule match and hostname
* Ensure all generated route conditions include a pattern for the hostnames (route selectors’, HTTPRoute’s or Gateway’s)
* Ensure all generated route conditions include the user-defined `when` conditions

* add tests for generating a wasmplugin config out of a complex httproute and complex ratelimitpolicy

The httproute declares 2 rules and 2 hostnames.

The RLP declares 2 limits:
- one limit declares one counter, targets one of the httprouterules, for one hostname only, with an additional 'when' condition and counter qualifier;
- the other limit declares 2 counters, targets the other httprouterule, for all hostnames, and states no additional 'when' condition nor counter qualifier.

* generate wasm rules only for routeSelector-filtered hostnames that belong to the targeted route

* fix: pattern expression for hostnames with the right operator - introduces 'endswith' pattern operator

* fix: compute wasm rules only for the applicable hostnames

Passes a list of hostnames into the call to the function that generates the wasm rules for a given RLP, notoriously to compute the conditions when to apply each limit. The list of hostnames is the domain intersection between the hostnames specified in the HTTPRoute and its parent gateway for which the wasm config is being built. The route selection considers this as a boundary, so it doesn’t generate rules for the wrong gateway/hostname.

Fixes the computation of conditions when no HTTPRouteRule matches the route selectors.

* do not generate wasm condition patterns for the hostname when all apply

* do not repeat RLPs for each hostname within the wasm config

* `rlptools.HTTPRouteRulesFromRouteSelector` moved to `api/v1beta2/route_selectors.go` along with the `RouteSelector` type itself

+ no longer injecting a separate list of hostnames for avoiding building wasm rules for hostnames that do not apply, as this did not seem to belong to the route selector logic anyway.

* reconciliation of route selectors for RLPs targeting Gateways

Get all HTTPRoutes that are children of the Gateway and call the generation of the set of wasm rules (conditions and data for each limit) for each HTTPRoute

* match the gateway hostnames instead of the httproute ones when building the wasm rules for rlps that target a gateway

* refactor: make HTTPRouteRuleSelector.Selects return sooner if the HTTP method does not match

* refactor: ensure wasm rule conditions are generated out of selected HTTPRouteRules in the same order as the selectors select the rules

* Prevent the usage of routeSelectors in RLPs that target a Gateway

* do not generate conditions for gateway policy wasm rules from httproutes that have a rlp of its own and avoid building wasmplugins when there are no rules to apply

* avoid adding rlps without any rules to the wasm config

* fix: do not use gateway name nor host in the RL domain

Fixes the reconciliation of the Limitador CR, pairing it with the reconciled wasm config (WasmPlugin CR), so
- rate limit definitions won't be duplicated in the Limitador CR;
- limits can be defined crossing gateways and hostnames and yet be treated as the same limit in Limitador (case of simple RLPs that target HTTPRoutes with multiple Gateway parent refs)

* Limit names in the Limitador conditions and RL request data generated in the format: limit.<limit-name>__<hash>, where <limit-name> is sanitised to include only characters allowed by Limitador for the identifiers and <hash> is generated out of the original limit name to avoid breaking uniqueness of the name after sanitisation.

* fix: missing rebase changes & update istio deps

For some reason after rebase, there is some dependency issue. This was fixed by updating the istio dep to https://github.com/istio/istio/releases/tag/1.17.5

* fix: lint issues

* refactor: lint change for CountersAsStringList() and deleteLimits()

Co-authored-by: Guilherme Cassolato <[email protected]>

---------

Co-authored-by: Eguzki Astiz Lezaun <[email protected]>
Co-authored-by: Guilherme Cassolato <[email protected]>
Co-authored-by: KevFan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request size/medium
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants